Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade mongodb-client to 5.1.3 #42445

Merged
merged 1 commit into from
Aug 10, 2024
Merged

Upgrade mongodb-client to 5.1.3 #42445

merged 1 commit into from
Aug 10, 2024

Conversation

gcw-it
Copy link
Contributor

@gcw-it gcw-it commented Aug 9, 2024

The extension compiles now, and the integration test runs successfully.

The following changes were necessary:

  • changed/removed access to deprecated and removed classes/methods
  • adjusted graalvm substitutions

There were a couple of deprecations, that were ignored for too long and the corresponding classes/methods were removed in this or an earlier version of the mongoldb java client.

I believe there is still some work to do with the extension, because there are still more than 30 deprecation warnings during compilation.

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/mongodb labels Aug 9, 2024
@gcw-it gcw-it mentioned this pull request Aug 9, 2024
Closed
gsmet
gsmet reviewed Aug 9, 2024
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

I added some small questions inline.

BTW, if you feel like it and things are relatively straightforward, I'm all for removing the calls to deprecated methods.
It might be too involved though so feel free to not do it.
Probably a good idea to do it in a specific commit if you pursue this.

extensions/mongodb-client/runtime/src/main/java/io/quarkus/mongodb/FindOptions.java
Comment on lines -297 to -299
if (oplogReplay) {
publisher = publisher.oplogReplay(true);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also drop the field/method in our class?

It doesn't have any replacement that we would need to support?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your attentiveness, yes we should drop the field. I'll take care of it.

According to the 4.11.1 Javadocs this option isn't supported on a database level from MongoDB 4.4 onwards.

No additional options were introduced between the versions.

extensions/mongodb-client/runtime/src/main/java/io/quarkus/mongodb/MapReduceOptions.java
Comment on lines -28 to -29
private boolean sharded;
private boolean nonAtomic;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no replacement or additional fields we need to support?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the 4.11.1 Javadocs these two options aren't supported on a database level from MongoDB 4.4 onwards.

In comparison to 4.11.1 only those two fields were removed and none were added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe there will be more work that needs to be invested in this area in the future.
The whole Map-Reduce functionality was deprecated in version 5.0 and should be replaced with aggregates.
See Map-Reduce.

@gcw-it
Copy link
Contributor Author

gcw-it commented Aug 9, 2024

removed the superfluous field.

@gsmet
Copy link
Member

gsmet commented Aug 9, 2024

Sorry to be that guy but could you have a first line in the commit message saying it's the upgrade? (i.e. the title of the PR)

@gcw-it
Copy link
Contributor Author

gcw-it commented Aug 9, 2024

No problem.

@gsmet
Copy link
Member

gsmet commented Aug 9, 2024

Perfect. In general, no problem to add a lot more details to the message but the first line should be really the top level thing we are doing.

gsmet
gsmet approved these changes Aug 9, 2024
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Let's run CI and see how it goes.

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 9, 2024
@gsmet gsmet marked this pull request as ready for review August 9, 2024 14:18
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Aug 9, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 03e64b3.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging/deployment

✖ io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector - History

  • Expecting actual: ["-6","-7","-8","-10","-11","-12","-13","-14"] to start with: ["-6", "-7", "-8", "-9"] - java.lang.AssertionError
java.lang.AssertionError: 

Expecting actual:
  ["-6","-7","-8","-10","-11","-12","-13","-14"]
to start with:
  ["-6", "-7", "-8", "-9"]

	at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:41)

@gsmet gsmet merged commit 5b360d9 into quarkusio:main Aug 10, 2024
67 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Aug 10, 2024
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 10, 2024
@gcw-it gcw-it deleted the pr_mongoup branch August 10, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsmet gsmet gsmet approved these changes

Assignees
No one assigned
Labels
area/dependencies Pull requests that update a dependency file area/mongodb release/noteworthy-feature triage/flaky-test
Projects
None yet
Milestone
3.14.0.CR1
Development

Successfully merging this pull request may close these issues.
2 participants
@gcw-it @gsmet